Fix failed sub-asset loads getting stuck in LoadState::Loading
#22628
+204
−9
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… least for the case where the sub-asset type is known.
…t is not of the requested type.
… requested type".
…a `AssetLoadError` variant.
…d was requested on the sub-asset. This meant the `LoadStatus` of the sub-asset would not reflect the error.
greeble-dev
added
C-Bug
greeble-dev
added
the
S-Needs-Review
andriyDev
approved these changes
Jan 22, 2026
Comment on lines
+3000
to
+3006
| for _ in 0..LARGE_ITERATION_COUNT { | ||
| app.update(); | ||
| load_state = asset_server.get_load_state(&handle).unwrap().into(); | ||
| if load_state == expected_load_state { | ||
| break; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Should we just use run_app_until here instead?
Contributor
Author
There was a problem hiding this comment.
I did this initially, but found that it made debugging test failures harder - run_app_until will panic without any information about what failed. Doing it manually lets me print a clear "did X, expected Y, got Z".
I do think there's room for improvement. Maybe adding a run_app_until variant with more options, or even better a way to say "run until all loads have resolved". But don't think that would fit in this PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Fix #22607. Also add a test that reproduces the issues.
If it makes things clearer, I can split this into an "add a test" PR and a separate "fix the bug" PR.
Solution
There are three additions to
AssetServer::load_internalthat each fix a separate case. In source code order:Case 1
Previously this would silently fail - the asset would successfully load, but the handle can't actually resolve to that asset so it's stuck in limbo.
Fixed by adding a type check, then sending a
AssetLoadError::RequestedHandleTypeMismatchevent and returning it as an error.Note that this doesn't prevent someone loading the asset with the correct type - the error is only associated with the
<WrongAssetType>handle.Case 2
Previously this was detected and would return a
AssetLoadError::MissingLabelerror fromload_internal, but the load status for the handle wasn't set.Fixed by sending an
InternalAssetEvent::Failed.Case 3
Previously this was detected, but the event was sent with the root asset's id (
base_asset_id) so it wasn't associated with the sub-asset's handle.Fixed by using the sub-asset's id.
Notes
Some of the events are only sent if
asset_idis set. This might seem odd, but I think it's correct -asset_idcan only be unset when usingload_untypedwith a sub-asset path, in which case there's no handle to associate with the error, and so there's no handle to pass toget_load_status.Testing